-
Notifications
You must be signed in to change notification settings - Fork 875
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Implements the use of ProblemDetail for HTTP error responses. (issue #149 and PR #173) #175
base: master
Are you sure you want to change the base?
Implements the use of ProblemDetail for HTTP error responses. (issue #149 and PR #173) #175
Conversation
Could you please provide an example of an error response? |
private record ErrorInfo(String className, String exMessage) { | ||
public ErrorInfo(Exception ex) { | ||
this(ex.getClass().getName(), ex.getLocalizedMessage()); | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I suppose we have to rework the openapi.yml
file in order to reference the problem details schema instead of the RestError
one.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Of course, I'll send the commit with the changes in the openapi.yml, along with some improvements to the ProblemDetail itself.
src/main/resources/openapi.yml
Outdated
title: Type | ||
description: Full URL that originated the error response. | ||
type: string | ||
format: text |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
issue: we have to use theuri
format
I think you could use the schema of this repository: https://github.com/belgif/openapi-problem/blob/master/src/main/openapi/problem/v1/problem-v1.yaml
It looks like very complete
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would it just be changing the value of the "format" property from "text" to "uri"?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, as a minimum. I think we can take inspiration from what @pvdbosch with his agreement
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I will implement this change in the "format" property and maybe a few more according to the @pvdbosch.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi, OK to use from the belgif openapi-problem repository! License is the same (both Apache License 2.0), so no problem there.
src/main/resources/openapi.yml
Outdated
description: The long error message. | ||
type: string | ||
example: 'Request failed schema validation' | ||
example: '2024-11-23T13:59:21.3820407' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Time offset is mandatory in the date-time format (which uses https://www.rfc-editor.org/rfc/rfc3339#section-5.6 ), so I believe this should be something like '2024-11-23T13:59:21.3820407Z' or '2024-11-23T13:59:21.3820407+00:00'
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
O deslocamento de tempo é obrigatório no formato de data e hora (que usa https://www.rfc-editor.org/rfc/rfc3339#section-5.6 ), então acredito que deveria ser algo como '2024-11-23T13:59:21.3820407Z' ou '2024-11-23T13:59:21.3820407+00:00'
Hello @pvdbosch, I could implement it without any problems
Implements the use of ProblemDetail according to issue #149, separating it from other changes according to PR #173.